Add TaskExecutor conformance to EventLoops #2732
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fabianfett
force-pushed
the
ff-nio-task-executor
branch
2 times, most recently
from
c48fcba to
a982359
Compare
Lukasa
reviewed
May 29, 2024
| // MARK: TaskExecutor conformance | ||
| #if compiler(>=6.0) | ||
| @available(macOS 9999.0, iOS 9999.0, watchOS 9999.0, tvOS 9999.0, *) | ||
| extension SelectableEventLoop: NIOTaskEventLoopExecutor { } |
There was a problem hiding this comment.
Can you also add conformance for AsyncTestingEventLoop and a fatalErroring conformance for EmbeddedEventLoop?
fabianfett
commented
May 29, 2024
| let loop1 = iterator.next()! | ||
| let loop2 = iterator.next()! | ||
|
|
||
| await self._runTests(loop1: loop1, loop2: loop2) |
There was a problem hiding this comment.
this test currently crashes:
Thread 4 Crashed:: NIO-ELT-0-#0
0 libswiftCore.dylib 0x19a319644 swift_getObjectType + 88
1 libswift_Concurrency.dylib 0x2617ea0e4 swift_task_enqueueImpl(swift::Job*, swift::SerialExecutorRef) + 160
2 libswift_Concurrency.dylib 0x2617ec890 swift::AsyncTask::flagAsAndEnqueueOnExecutor(swift::SerialExecutorRef) + 432
3 libswift_Concurrency.dylib 0x2617eacdc swift_task_switchImpl(swift::AsyncContext*, void (swift::AsyncContext* swift_async_context) swiftasynccall*, swift::SerialExecutorRef) + 640
4 swift-nioPackageTests 0x1080d5ca1 partial apply for closure #1 in closure #1 in TaskExecutorTests._runTests<A, B>(loop1:loop2:) + 1
5 swift-nioPackageTests 0x1080d4d3d thunk for @escaping @isolated(any) @callee_guaranteed @Sendable @async () -> (@out A) + 1
6 swift-nioPackageTests 0x1080d4e99 partial apply for thunk for @escaping @isolated(any) @callee_guaranteed @Sendable @async () -> (@out A) + 1
7 libswift_Concurrency.dylib 0x2617f1921 completeTaskWithClosure(swift::AsyncContext*, swift::SwiftError*) + 1
Why do we call swift_task_switchImpl with a SerialExecutorRef?
There was a problem hiding this comment.
switch is what checks if it can "switch" without releasing actor locks; it won't be able to switch when there's a task executor. I'll look into reproducing this though with a debug runtime to check.
|
Can we also add another benchmark for the |
fabianfett
force-pushed
the
ff-nio-task-executor
branch
from
ba8f9fb to
6018f07
Compare
|
Performance results on my machine: |
fabianfett
force-pushed
the
ff-nio-task-executor
branch
from
7d76649 to
8ec67a1
Compare
fabianfett
force-pushed
the
ff-nio-task-executor
branch
from
8ec67a1 to
cb67cb2
Compare
|
Updated the PR for 6.1: Looks like the numbers got worse. |
|
I hope that this PR will improve the numbers: |
Lukasa
requested changes
Apr 9, 2025
I just want to expand on this in case anyone sees this comment and assumes they should avoid task executors. That is certainly not the case. Task executors and region based isolation work together nicely; however, the problem is that swift concurrency has no way to represent a value that’s tied to a specific thread unless that thread is modeled with an isolation domain (actor or Sendable closures). This is a common pattern in NIO though which often makes runtime checks that code is executing in the right event loops before allowing changes or returning event loop bound state. Doing this directly from asynchronous swift code is only possible when using custom actor executors, or task executors or by hooking the global executor. Arguably we have already allowed two (actor and global) of the three potentially dangerous patterns here. So in general when writing pure Swift concurrency code using task executors with region isolation is completely safe. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add TaskExecutor conformance to EventLoops
Motivation
Swift 6 introduces the option to supply your own task executors to the Swift concurrency runtime (SE-0417). SwiftNIO's EventLoop should conform to the new TaskExecutor protocol to support running Swift concurrency Tasks on it. This will allow fewer context switches in Swift on server applications.
Modifications
SerialExecutor)NIODefaultSerialEventLoopExecutortoNIODefaultEventLoopExecutorNIODefaultEventLoopExecutornow also implementsTaskExecutorprotocolNIOTaskEventLoopExecutorprotocol that makes it easy for adopters to get a better implementationResult
Adopters can use ELs as task executors